-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use quote_plus instead of quote #122
Conversation
`asyncpg` doesn't work well for all US-ASCII characters, we need to use `quote_plus` instead MagicStack/asyncpg#1159
@ranchodeluxe I'm trying to reproduce the issue using a password like FYI: I'm changing https://github.com/stac-utils/stac-fastapi-pgstac/blob/main/tests/conftest.py#L56 to |
Sorry for the late reply @vincentsarago. I couldn't run tests here a few weeks ago gave up. The For what it's worth still can't run the test docker image locally. It's failing to pip install things correctly in site-packages for the image user "newuser". Haven't spent much time on it but can try later |
Okay, just approve the workflow please and we should see tests fail 😉 |
@@ -1,7 +1,7 @@ | |||
"""Postgres API configuration.""" | |||
|
|||
from typing import List, Type | |||
from urllib.parse import quote | |||
from urllib.parse import quote_plus as quote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-added this after confirming locally that the test were failing!
connection = ( | ||
f"postgresql://{jan.user}:{jan.password}@{jan.host}:{jan.port}/{jan.dbname}" | ||
) | ||
connection = f"postgresql://{jan.user}:{quote(jan.password)}@{jan.host}:{jan.port}/{jan.dbname}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the test we also needed to quote
the password.
Yeah sorry about that, I though I fixed this but it doesn't seems to be the case. for what is worth we've made it easier to just run the test locally (in a virtual env) because now it doesn't touch your local postgres instance. Once all the test are ✅ I'll merge and update the changelog. thanks @ranchodeluxe 🙏 |
asyncpg
doesn't work well for all US-ASCII characters, we need to usequote_plus
insteadRelated Issue(s):
MagicStack/asyncpg#1159
Description:
See PR and related ticket above
PR Checklist:
pre-commit
hooks pass locallymake test
)make docs
)